Skip to content

chore(selfhost): remove dead nonConsumingRetryDelayMs export - #6357

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
galuis116:chore/remove-dead-non-consuming-retry-delay
Jul 16, 2026
Merged

chore(selfhost): remove dead nonConsumingRetryDelayMs export#6357
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
galuis116:chore/remove-dead-non-consuming-retry-delay

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Closes #6287

Summary

  • Remove nonConsumingRetryDelayMs from src/selfhost/queue-common.ts — a trivial pass-through alias to githubRateLimitRetryDelayMs with zero real (non-test) callers anywhere in the repo. Its sibling consumingRetryDelayMs is the one actually used by both queue backends.
  • Remove its dedicated test (selfhost-queue-common.test.ts's "keeps only GitHub rate limits on the non-consuming retry path") and the now-unused import.

Scope

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck — root tsc --noEmit OOMs on this shared sandbox regardless of branch (reproduced identically on a clean main checkout via git stash). No substitute typecheck was needed here beyond what vitest's esbuild transform already catches, since the change is a pure removal (no new type surface) confirmed by a repo-wide grep showing zero remaining references.
  • npm run test:coverage — not run repo-wide (same OOM risk), but the full set of tests importing from queue-common.ts was run directly and passes: selfhost-sqlite-queue.test.ts, queue-5.test.ts, selfhost-queue-common.test.ts, queue-2.test.ts, selfhost-pg-queue.test.ts, queue-4.test.ts, queue.test.ts, queue-3.test.ts, selfhost-installation-concurrency-admission.test.ts, queue-lifecycle-guards.test.ts — 1273/1273 passing. (index.test.ts in the same run fails on a pre-existing, unrelated Cannot find package '@sentry/hono/cloudflare' module-resolution error in src/api/routes.ts/src/index.ts — reproduced identically on a clean main checkout with this diff stashed, so it predates and is unrelated to this change.) This PR is a pure deletion of dead code and its own test, so patch coverage is inherently 100% (no new lines added).
  • npm run test:workers — N/A, no Worker-facing behavior changed.
  • npm run build:mcp / npm run test:mcp-pack — N/A, no @loopover/mcp changes.
  • npm run ui:openapi:check — N/A, no API/OpenAPI changes.
  • npm run ui:lint / npm run ui:typecheck / npm run ui:build — N/A, no apps/loopover-ui changes.
  • npm audit --audit-level=moderate — 0 vulnerabilities.
  • New/changed behavior has tests — N/A, this removes dead code and its dedicated test; no new behavior.

If any required check was skipped, explain why:

  • Root npm run typecheck / npm run test:coverage: this sandbox runs on a machine shared with other concurrent sessions; the root tsc --noEmit reliably OOMs under that memory pressure regardless of what changed (reproduced identically with this branch's diff fully stashed against main). Substituted with a full repo-wide grep confirming zero remaining references to the removed export, plus every directly-importing test file run and passing (1273/1273).

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A, no such changes.
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI changes.
  • Visible UI changes include a UI Evidence section below with screenshots. — N/A, no visible UI change.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. — CHANGELOG.md untouched.

Notes

  • Confirmed via grep -rln "nonConsumingRetryDelayMs" src/ (excluding its own definition file) that no production code imports it, matching the issue's finding exactly.

A repo-wide search confirms its only reference besides its own
definition was its dedicated test — the sibling consumingRetryDelayMs
is the one actually used by both queue backends.

Closes JSONbored#6287
@galuis116
galuis116 requested a review from JSONbored as a code owner July 16, 2026 04:03
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@loopover-orb

loopover-orb Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-16 04:13:11 UTC

2 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a clean, narrowly-scoped dead-code removal: `nonConsumingRetryDelayMs` was a trivial pass-through to `githubRateLimitRetryDelayMs` with no production callers, and the reference context confirms both `src/index.ts` and `src/queue/dlq.ts` only import other queue-common helpers, not this one. The PR correctly removes the function, its import in the test file, and the dedicated test that only exercised it, and is properly linked to an open issue (#6287).

Nits — 3 non-blocking
  • The PR description notes root `tsc --noEmit` and `test:coverage` couldn't be run repo-wide due to a pre-existing OOM/module-resolution issue unrelated to this change — worth a maintainer sanity-check but not a defect in the diff itself.
  • A repo-wide grep output (e.g., `grep -rn nonConsumingRetryDelayMs`) in the PR description would make the 'zero remaining references' claim independently verifiable without re-running it.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6287
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 1920 registered-repo PR(s), 1265 merged, 54 issue(s).
Contributor context ✅ Confirmed Gittensor contributor galuis116; Gittensor profile; 1920 PR(s), 54 issue(s).
Improvement ℹ️ None detected risk: low · value: none · LLM: minor
Linked issue satisfaction

Addressed
The PR removes the dead nonConsumingRetryDelayMs export from queue-common.ts along with its dedicated test and import, exactly matching the issue's requirements and deliverable.

Review context
  • Author: galuis116
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 1920 PR(s), 54 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #6172, issue #6253)
  • Related work: Titles/paths share 7 meaningful terms. (issue #6172, issue #6287)
  • Related work: Titles/paths share 7 meaningful terms. (issue #6182, issue #6253)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 6870c87 into JSONbored:main Jul 16, 2026
14 checks passed
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (d5d8da8) to head (be55115).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6357      +/-   ##
==========================================
- Coverage   95.60%   95.60%   -0.01%     
==========================================
  Files         598      598              
  Lines       47202    47201       -1     
  Branches    15022    15022              
==========================================
- Hits        45128    45127       -1     
  Misses       1290     1290              
  Partials      784      784              
Flag Coverage Δ
shard-1 43.97% <ø> (-0.18%) ⬇️
shard-2 36.80% <ø> (+0.41%) ⬆️
shard-3 32.53% <ø> (+<0.01%) ⬆️
shard-4 34.43% <ø> (-0.19%) ⬇️
shard-5 30.99% <ø> (-0.59%) ⬇️
shard-6 45.30% <ø> (+0.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/selfhost/queue-common.ts 98.65% <ø> (-0.01%) ⬇️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(selfhost): remove dead nonConsumingRetryDelayMs export in queue-common.ts

1 participant